[DA] getBackedgeTakenCount in isKnownLessThan can return CouldNotCompute #162495
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bail out when the backedge taken count is a CouldNotCompute SCEV in function isKnownLessThan; we cannot and do not want to query things like its Type.
llvmbot
added
the
llvm:analysis
|
@llvm/pr-subscribers-llvm-analysis Author: Sjoerd Meijer (sjoerdmeijer) ChangesBail out when the backedge taken count is a CouldNotCompute SCEV in function isKnownLessThan; we cannot and do not want to query things like its Type. Full diff: https://github.com/llvm/llvm-project/pull/162495.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 1f0da8d1830d3..9e474769abd83 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -1184,6 +1184,8 @@ bool DependenceInfo::isKnownLessThan(const SCEV *S, const SCEV *Size) const {
if (const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(S))
if (AddRec->isAffine() && AddRec->hasNoSignedWrap()) {
const SCEV *BECount = SE->getBackedgeTakenCount(AddRec->getLoop());
+ if (isa<SCEVCouldNotCompute>(BECount))
+ return false;
const SCEV *Start = AddRec->getStart();
const SCEV *Step = AddRec->getStepRecurrence(*SE);
const SCEV *End = AddRec->evaluateAtIteration(BECount, *SE);
diff --git a/llvm/test/Analysis/DependenceAnalysis/becount-couldnotcompute.ll b/llvm/test/Analysis/DependenceAnalysis/becount-couldnotcompute.ll
new file mode 100644
index 0000000000000..4828858897072
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/becount-couldnotcompute.ll
@@ -0,0 +1,25 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Test for function isKnownLessThan that calculates a back-edge taken count,
+; which can return a CouldNotCompute SCEV.
+
+define void @test(i64 %conv, ptr %a) {
+; CHECK-LABEL: 'test'
+; CHECK-NEXT: Src: %.pre.pre.pre = load i32, ptr %arrayidx12, align 4 --> Dst: %.pre.pre.pre = load i32, ptr %arrayidx12, align 4
+; CHECK-NEXT: da analyze - none!
+;
+entry:
+ %sub = add i64 %conv, 1
+ br label %for.cond
+
+for.cond:
+ %i.0 = phi i64 [ %add26, %for.cond ], [ 0, %entry ]
+ %arrayidx12 = getelementptr i32, ptr %a, i64 %i.0
+ %.pre.pre.pre = load i32, ptr %arrayidx12, align 4
+ %add26 = add nsw i64 %sub, %i.0
+ br label %for.cond
+}
|
kasuga-fj
reviewed
Oct 9, 2025
There was a problem hiding this comment.
Could you please clarify the motivation for this change? When I tried the test case you added, it crashed. So I guess this PR is meant to address that issue, right?
Side note: Although fixing such a bug is fine to me, I believe this function has multiple correctness issues and should be removed in the first place.
Comment on lines
+4
to
+5
| target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32" | ||
| target triple = "aarch64-unknown-linux-gnu" |
There was a problem hiding this comment.
Suggested change
| target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32" | |
| target triple = "aarch64-unknown-linux-gnu" |
Maybe unnecessary?
Comment on lines
+15
to
+25
| entry: | ||
| %sub = add i64 %conv, 1 | ||
| br label %for.cond | ||
|
|
||
| for.cond: | ||
| %i.0 = phi i64 [ %add26, %for.cond ], [ 0, %entry ] | ||
| %arrayidx12 = getelementptr i32, ptr %a, i64 %i.0 | ||
| %.pre.pre.pre = load i32, ptr %arrayidx12, align 4 | ||
| %add26 = add nsw i64 %sub, %i.0 | ||
| br label %for.cond | ||
| } |
There was a problem hiding this comment.
I think this can be simplified. If you just want to write an infinite loop, some values (%i.0, %conv, etc.) should not be necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bail out when the backedge taken count is a CouldNotCompute SCEV in function isKnownLessThan; we cannot and do not want to query things like its Type.